Skip to content

Detect diamond#77

Open
ddifi wants to merge 43 commits into
gobo-eiffel:masterfrom
ddifi:detect_diamond
Open

Detect diamond#77
ddifi wants to merge 43 commits into
gobo-eiffel:masterfrom
ddifi:detect_diamond

Conversation

@ddifi

@ddifi ddifi commented Mar 8, 2024

Copy link
Copy Markdown

This problem is more serious than we have thought:

#70 (comment)

Implemented step-wise field path:

#70 (comment)

@ebezault

ebezault commented Mar 9, 2024

Copy link
Copy Markdown
Collaborator

I cannot accept this pull request as it is. Too many issues to report them one by one in the PR. In summary:

  • The description of the class mentions implicit conversions, not attribute renaming in a diamond.
  • The class contains a lot of unused code copy/pasted from the implicit converts format.
  • The class contains commented out code.
  • New features are missing head comments and assertions.
  • The Gobo project does not use Makefiles but is relying on geant.
  • The logic provided in the Python file should be implemented in Eiffel in class GEDOC_FIELD_RENAME_FORMAT.
  • Use Eiffel terminology, e.g. in Eiffel we don't talk about fields but attributes.
  • The rename test should use getest. It should go into an example folder. Its code does not follow the Eiffel standards (bad formatting, is keyword is deprecated, the use of () violates the uniform access principle, ...).

@mw66

mw66 commented Mar 9, 2024

Copy link
Copy Markdown

Thanks for looking into this PR. Sorry, I forget to mention that I just want to get your first review opinion (I created the PR rather late in the night at my time, and I was tired and sleepy).

I will fix the concerns you had.

Actually, do you have a better name for the proposed FIELD_RENAME flag?

Another thing I want to discuss beforehand is: do your really think the use of Python is a concern?

The CSV output is easier for the programmers to inspect, and Python has good support for CSV files and file system traversal (i.e

  ecf_fns = glob.glob('**/*.ecf', recursive=True)

)

I'm not sure how easily these things can be done in Eiffel.

(I want the tool to work in small steps, so each step can be inspected and debugged; instead of just one pass, which complicate the development process for novice like me.)

As you can easily tell, my Eiffel programming skill is very limited: from the elementary syntax, the Eiffel library api, and to the GOBO Eiffel compiler's inner working. I only wrote some simple Eiffel code ~20 years ago.

With all these developer constraints, would you consider take the Python script?

Or, would you like to take my implementation just as the new detector's prototype, and start from scratch and do everything in the Eiffel way? I'd be happy if you can help, or just takeover what I have done from here, and make it a proper tool for the Eiffel users as soon as we can, since we know it's a real problem in Eiffel's attribute renaming semantics in real code.

What do you think?

@ebezault

ebezault commented Mar 9, 2024

Copy link
Copy Markdown
Collaborator

Another thing I want to discuss beforehand is: do your really think the use of Python is a concern?

During the past almost 30 years, I received many contributions like yours. The "problem" with contributions is that when it comes time to maintenance the contributors are not there anymore. At some point I spent more time maintaining the contributions from others than working on my own code. I'm not paid for that, so when that happens I prefer to work with a language that I like and which is the main reason why I started the Gobo project in the first place.

So yes, Python is a concern, and Makefile is a concern.

Or, would you like to take my implementation just as the new detector's prototype, and start from scratch and do everything in the Eiffel way? I'd be happy if you can help, or just takeover what I have done from here, and make it a proper tool for the Eiffel users

I can take over, but I won't have time to work on it before June or July.

as soon as we can, since we know it's a real problem in Eiffel's attribute renaming semantics in real code.

With that respect, I agree with Bertrand Meyer: if only a very small number of the Eiffel programs suffer from this problem, then the priority might be to focus on other problems or missing features that are more important for a larger number of Eiffel users.

@mw66

mw66 commented Mar 9, 2024

Copy link
Copy Markdown

So yes, Python is a concern, and Makefile is a concern.

No problem, fully understand.

I can take over, but I won't have time to work on it before June or July.

Thanks for taking over. Appreciated.

@mw66

mw66 commented Mar 9, 2024

Copy link
Copy Markdown

BTW, I'm not paid for any of these effort either. We all are just looking for a better / perfect programming language, and Eiffel is coming close, but didn't make it, esp. in terms of user adoption.

@mw66

mw66 commented Mar 10, 2024

Copy link
Copy Markdown

is keyword is deprecated

This is to make the code can be tested with the last ECMA compliant SmartEiffel compiler.

@ebezault

Copy link
Copy Markdown
Collaborator

is keyword is deprecated

This is to make the code can be tested with the last ECMA compliant SmartEiffel compiler.

I don't think we should care about a compiler version which is almost 20 years old.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants